Skip to content

fix(agc): harden host import construction and mutation waits - #2779

Closed
nikhilpanchal1 wants to merge 1 commit into
boykopovar:mainfrom
nikhilpanchal1:fix/host-import-retirement
Closed

nikhilpanchal1 wants to merge 1 commit into
boykopovar:mainfrom
nikhilpanchal1:fix/host-import-retirement

Conversation

@nikhilpanchal1

Copy link
Copy Markdown
Contributor

What

Follow-up to #2575, which merged MarcZX100's fix for #2352. Clean up incomplete Vulkan host imports before releasing their allocation pins. Install the mutation waiter centrally for descriptor-first imports, release completed batch holds during nested waits, reject pinned mutations in active completion callbacks, and propagate waiter errors after restoring the registry lock.

#1357, #1359 and #1361 cover related retirement ownership; that work is excluded. #1961 installs a waiter for its new map-time imports. This change covers existing descriptor-first imports centrally. #2283 changes kept-byte accounting; #290 adds import windows, eviction and memory-pressure release waits. Its pressure relief returns when the caller holds the GPU lock. These PRs touch related functions but do not implement the remaining safeguards here.

Tested

Linux x86-64 Release, Mesa 26.1.6 llvmpipe, Vulkan required. Full build and runtime library patching pass. Eight isolated cases fail main at 5edae353 and pass here; three controls pass both. Native guest APIs and a relinker-converted ELF exercise real Vulkan fills/copies and memory mutations, including Linux heap cache reuse and uncached reset/release. All nine runnable new tests pass 50 times with the shader cache disabled: 450 executions.

Full suite: 670 passed, 37 skipped, two failed. All 698 existing tests retain their baseline results, including the same f32 denormal and f64 FMA failures. Two new write-watch tests skip because userfaultfd is unavailable; the skip audit rejects those plus existing agc_write_tracking. Conventions and test registration pass. Windows, NVIDIA, DMA-buf chunks and Windows heap caching remain untested.

Checklist

  • Based on main at 5edae353; related open PRs reviewed and function overlaps disclosed above
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except existing technical debt
  • Unimplemented paths throw; no silent stubs added
  • No notes, investigation files or images added to the repository
  • No new third-party code
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

Guard incomplete imports and preserve mutation waiter errors and progress.

Follow-up to boykopovar#2575; refs boykopovar#2352.
@github-actions github-actions Bot added the intake paused Closed while PR intake is limited to authors with at least 10 PRs merged into main label Oct 10, 2026
@github-actions

Copy link
Copy Markdown

Thank you for your interest in the project! We are sorry, but we cannot accept more PRs while we work through the existing backlog. This PR has been automatically closed and labeled "intake paused". This is not a judgment of the quality of your changes.

@zaydzaari

Copy link
Copy Markdown
Contributor

Windows/NVIDIA validation — RTX 5070

Tested PR #2779 on Windows 11 using an NVIDIA RTX 5070 and MinGW-w64 GCC 15.2.0.

Revisions

  • Baseline: 5edae35361e9b7f7061f28c6e80ed075312b8105
  • PR: db76c694378824b950a74565ec4aa458d52db5cf
  • NVIDIA driver: 610.47 (Vulkan 1.4.341, Subgroup size 32)

Results

  • Targeted tests: 15 passed, 0 failed, 2 skipped (unit shadow tests cleanly exit code 77 due to Windows memory-comparison host import model)
  • Baseline defect reproductions:
    • agc_driver_host_import_lifetime: failed (timeout waiting 2000 ms for lease; unreleased kept lease in deferred batches)
    • agc_driver_host_import_mutation_race: failed (registry lock left unlocked on pin waiter exception)
  • PR-side results: both lifetime and mutation-race tests pass cleanly
  • Repeated stress tests: 80/80 passed (20 iterations each across host_import_lifetime, host_import_mutation_race, guest_memory_lifetime, and host_import_teardown)

Notable observations

  • The deferred-keep lease timeout during synchronous waits was reproduced on baseline both via dynamic PRX redirection and native test compilation, and is resolved by Recorder::ReleaseCompletedKeeps().
  • The registry mutex leak on pin-waiter exceptions was reproduced on baseline and is resolved by exception-safe lock recovery.
  • No Vulkan device losses (VK_ERROR_DEVICE_LOST), crashes, or timeouts were observed across all 80 executed stress runs on the RTX 5070.

Limitations

  • Validation is specific to Windows 11 / NVIDIA RTX 5070 / MinGW-w64 GCC 15.2.0.
  • Unit shadow tests skip on Windows as host imports do not support page write-watching on this platform.
  • Full AnyPS5 emulator integration suite was not executed; coverage is confined to the targeted graphics and memory driver suites.
  • No heap profiler measurements were taken to formally evaluate memory consumption over extended workloads.

Conclusion
The evidence on Windows 11 / NVIDIA RTX 5070 confirms that PR #2779 fixes the reproduced lease timeout and registry lock inversion bugs without introducing observed regressions in the tested AGC driver paths. Full validation logs and per-iteration records are preserved locally.

@popcoder565

Copy link
Copy Markdown
Contributor

Windows/NVIDIA validation, as asked on #2575. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head db76c694, merged onto main ca34953 (together with justdoston's select/poll branch from #1471, which touches only Socket.cpp, Udp.cpp and GuestPoll.cpp).

  • All ten lifetime and import tests run, with none skipped, and pass in 20 of 20 rounds: agc_host_import_unmap, agc_driver_host_import_lifetime, agc_driver_host_import_mutation_race, agc_driver_host_import_teardown and the six agc_driver_guest_memory_lifetime* variants (copy, direct, locked, heap, heap_uncached).
  • Full ctest: 719 of 719 in two of three runs. In the other run, agc_driver failed once with 0xc0000374 under -j 8. It then passed 10 of 10 on its own, and I haven't measured its rate on main, so I can't tell whether it relates to this change.

🤖 Addressed by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

intake paused Closed while PR intake is limited to authors with at least 10 PRs merged into main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants